Skip to content
This repository was archived by the owner on Oct 26, 2018. It is now read-only.

Restore location after devtools reset #6

Merged

Conversation

ellbee
Copy link
Member

@ellbee ellbee commented Nov 6, 2015

I found a problem with Devtools where a 'RESET' will change the path to '/' regardless of the initial URL. This is the quick workaround I am currently using, but I wonder if there is a better way.

@jlongster
Copy link
Member

Good catch! I will test it out myself as well and think about it. I'd like if it we didn't have to explicitly handle this, but we'll see. Might take me a day or two to look at this more deeply.

@ellbee
Copy link
Member Author

ellbee commented Nov 7, 2015

Thanks, that sounds good, it would definitely be preferable to not have this code in the project!

Actually, my initial thought with this was just to set the initialState of routerReducer.path to locationString(window.location) when running in a browser. For some reason that eludes me now I decided it wouldn't work, but having thought about it some more it seems like it would work OK:

const initialState = typeof window === undefined ? {} : {
  path: locationToString(window.location)
};

@jlongster
Copy link
Member

@ellbee oh, good point! I don't see any problem with that. Can you test it and see if it works, and possibly update this PR? If not I can get to it this week.

@ellbee
Copy link
Member Author

ellbee commented Nov 9, 2015

Sure, I'll have some time to look at it later today.

@ellbee ellbee force-pushed the restore-location-after-devtools-reset branch from eb9e799 to c5681bd Compare November 9, 2015 13:17
@ellbee
Copy link
Member Author

ellbee commented Nov 9, 2015

I've updated the PR. It is working fine in the tests I have done.

@jlongster
Copy link
Member

Looks good, thanks!

jlongster added a commit that referenced this pull request Nov 10, 2015
@jlongster jlongster merged commit 1216ad1 into reactjs:master Nov 10, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants